Repository navigation
Conversation
samgbafa
commented
Oct 15, 2026
samgbafa
left a comment
Contributor
Author
There was a problem hiding this comment.
APPROVE — TC-735
Contributor
Author
|
REQUEST_CHANGES: see TC-735 |
samgbafa
commented
Oct 15, 2026
samgbafa
left a comment
Contributor
Author
There was a problem hiding this comment.
REQUEST_CHANGES — TC-735
Contributor
Author
|
APPROVE: see TC-735 |
samgbafa
commented
Oct 15, 2026
samgbafa
left a comment
Contributor
Author
There was a problem hiding this comment.
APPROVE — TC-735
Keep main's alias-aware revocation::effective_rows after the batched snapshot (an extra round trip) so this merge alone preserves TC-1071; the next commit folds it into the single statement. Production locking now uses main's load_chain_lock_ids; load_closure_edges is a test oracle.
TC-1071 revokes every stored encoding sharing a canonical identity. Add an alias-joined revocation branch to the single snapshot statement, mirroring revocation::effective_rows, and drop the merge's extra query. Regression: a revocation under another encoding is refused on SQLite, PostgreSQL and MySQL. A seeded property test compares production batched decisions with the sequential effective_rows snapshot over random DAGs, alias groups, revoked and expired delegations.
Contributor
Author
|
APPROVE: see TC-735 (head 580abe5) |
samgbafa
added a commit
that referenced
this pull request
Oct 17, 2026
Brings main a735c4d (TC-1071 CID-alias revocations, TC-1176 boot lock_timeout, request telemetry) and #302 580abe5, whose batched auth snapshot resolves alias revocations in its one statement. Startup keeps the pre-S3-listing migration (now main's lock_timeout- bounded migrate) so deleting fences are read before the quota listing, then opens the database through main's new_with_encryption, which finds migrations current and runs the CID-alias backfill with the column key before serving requests.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
Fetch authorization closure edges, delegations, abilities and revocations in one tagged SQL query; rebuild the snapshot in Rust. Capability validation, revocation rules and schema stay unchanged. This PR is required before TC-735's pre-persist write-path PR #301.
The metadata joins use a planner-visible 65-node (64 valid + overflow sentinel) lookup relation. The full closure and edge budget checks remain separate; 64 nodes load completely and 65/66 fail closed. Ability rows have a canonical typed
(resource, ability)order across databases. Allow/deny outcomes do not change; only the diagnostic reason when several caveats all reject may normalize from the former backend-dependent order. SDK/CLI do not assert that graph rejection text; existing node containment-vector codes test the separate containment helper.Main integration and TC-1071 alias revocations
Main (
a735c4d) made revocation lookups alias-aware (TC-1071): revoking any stored encoding of a delegation revokes every stored encoding with the same canonical identity (revocation::effective_rows). The previous single statement joinedrevocation.revokeddirectly, and git merged that SQL without a conflict, so a naive merge would have accepted a delegation revoked under another encoding.80e74aemerges main. It keeps main'seffective_rowscall after the snapshot (correct, one extra round trip), so no commit in this branch is alias-blind. Production chain locks use main'sload_chain_lock_ids;load_closure_edgesis now test-only. This intermediate commit was not tested on its own; its one-round-trip assertion is expected to fail there.580abe5adds an alias-joined revocation branch to the same statement (lookup_nodes → delegation_cid_alias source → same-canonical target → revocation.revoked), mirroringeffective_rows, and removes the extra query. Signed revocation targets and stored IDs are unchanged.Evidence
All at
580abe562cf20c06b32a5124e76f0c0502ee30d8(Sam Gbafa), local PostgreSQL 16.15, MySQL 8.0.46, MinIO; PostgreSQL 18.6 for plans.assert_alias_revocation_refused): chainleaf → mid → root;midand another stored encoding share a canonical alias; the revocation names the other encoding.revocation::is_revoked(mid)is true. Before580abe5's statement change the batched snapshot did not markmidrevoked on SQLite, PostgreSQL 16 and MySQL 8 (~/scratch/tc-735/pr302-alias-red.log); after, it reportsmidrevoked, the leaf's first revoked ancestor ismid, and the snapshot equals the sequential one.assert_batched_matches_sequential_decisions; 200 SQLite, 60 PostgreSQL and 60 MySQL cases): random DAGs of 1–6 delegations, alias groups including uncited stored encodings, revocations of cited and uncited encodings, and expired delegations. For each single root and the all-roots set it requires: batched snapshot == sequentialeffective_rowssnapshot; per-rootis_revoked/first_revoked_ancestorrefusal == per-node DBrevocation::is_revoked/first_revoked_ancestor; and the productionauthorize_holderresult == the same checks run on the sequential snapshot. It asserts alias-only refusals and both outcomes occurred. Run alone against the alias-blind statement, it fails with the seed in the message (~/scratch/tc-735/pr302-property-red.log).snapshot_load_is_one_round_trip_and_matches_sequentialpasses; wire budget sign-in 69, KV PUT 34, KV GET 16 (unchanged;~/scratch/tc-735/pr302-580abe5-wire.log).delegation_cid_alias_pkey,idx_delegation_cid_alias_canonicalandidx_revocation_revoked; no sequential scan of those tables, no temporary spill, 0.195/0.187 ms (~/scratch/tc-735/pr302-alias-plan-pg{16,18}-100k.txt).CIunset only for the privileged iptables fixture). Strict all-target Clippy and formatting pass (~/scratch/tc-735/pr302-merge-full-suite.log).delegation_cid_alias.Coordination
TC-1149/#298 principal/alias closure helpers are preserved through main. Requesting a fresh security review of the alias delta (
80e74ae..580abe5) plus the merge resolution. No merge or deployment requested; TC-1176 release hold applies.